Streaming Timeseries example #469
Conversation
|
Your changes were successfully integrated in the dev site, make sure to review the pages of the projects you touched before merging this PR. |
|
@droumis would you mind describing in more detail what "smoother streaming updates" means to you? |
|
@amaloney. Minimize latency between patch updates to make the streaming smoother. While we can't go below the data sampling interval, the primary constraint will likely be HoloViews, which might limit updates to once every 10-20 ms at best (just an estimate). Achieving this could involve buffering smaller chunks of data to enable smoother interpolation without noticeable lag. Additionally, ensuring the buffered DataFrame range stays consistent across all channels/curves could maybe help minimize delays between data arrival and plotting. |
|
@amaloney the test pipeline is broken due to some unrelated issue for which a fix has already been merged on I also saw you applied black/isort to the code in this PR. You may be wondering why we don't do that automatically. If not, ignore me :D But if so, the main reason is that we want to let the example author format the code the way they prefer, which can sometimes be more readable than when done automatically by a tool. In the end, when I review an example the only things I care about about code formatting are that it is not too weird and that the lines aren't too wide (painful to read on mobile). |
|
Really cool example! |
|
Your changes were successfully integrated in the dev site, make sure to review the pages of the projects you touched before merging this PR. |
There was a problem hiding this comment.
Nice exampled indeed :)
I have to main pieces of feedback:
- It'd be nice to have a bit more of neuroscience-y context and narrative. What are these EEG traces I'm looking at? What does the channel positions plot represent?
- The
StreamingAppclass is quite long and a bit hard to follow. It'd be nice if:- A few of the main concepts could be explained beforehand (
hv.DynamicMap,hv.streams.Buffer,pn.state.add_periodic_callback,pn.io.unlocked). - The class was refactored a bit. For example, the plot methods (
create_{},bare_{}) could be defined as functions before the class is defined, and could be called once separately to show what output they generate. The methods in the class would call these functions. Of course, only if this refactor isn't too much work!
- A few of the main concepts could be explained beforehand (
| " self.position_pane = pn.pane.HoloViews(self.bare_pos_plot())\n", | ||
| " self.main_streaming_pane = pn.pane.HoloViews(self.bare_stream_plot())\n", | ||
| "\n", | ||
| " if self.notebook:\n", |
There was a problem hiding this comment.
Instead of this I think you could use if not pn.state.served.
There was a problem hiding this comment.
I think this is a great suggestion. I didn't even know it existed, but it's definitely in the documentation https://panel.holoviz.org/api/state.html. Do you have examples that use this feature? If so, I think it would be nice to show some links to it in this notebook.
There was a problem hiding this comment.
I use pn.state.served quite a lot in some code bases for the code to follow a different branch, usually to distinguish between the app being served and the test suite running.
Though I just remembered there were some subtleties with it (see more in holoviz/panel#5623). Since no other example use it and it's yet another thing to explain in a rather complex example, I'd say forget about my suggestion and let's stick with the more explicit notebook attribute?
There was a problem hiding this comment.
Sure, although I would personally like more info into its use as it looks like it could be a great flag to use. I'll see if I can put something on our calendar.
There was a problem hiding this comment.
yea, using a notebook flag is super hacky and I'd much prefer if there was a way to detect with pn.state.served, however, given the timeline, let's just come back to this in a later PR
| }, | ||
| "outputs": [], | ||
| "source": [ | ||
| "nb_app" |
There was a problem hiding this comment.
It'd be really cool if we could run the app a bit while testing and building the notebook. I've tried adding this in the next cell and it seemed to work. At least with the CPU source, the other one is a bit more brittle.
if not pn.state.served:
nb_app.start_stream()
import asyncio
await asyncio.sleep(5)
nb_app.pause_stream()
There was a problem hiding this comment.
I think this is for debugging, and not so much for the tutorial. I like it a lot, and will use it personally so thank you for it.
There was a problem hiding this comment.
it might be cool if we could have it trigger the cpu stream for a bit but only in the website build and otherwise hide the code from the tutorial notebook. But I don't have time to figure that out and also idk what it might even show for CPU usage on a CI system. So, dropping given urgency of initial release.. maybe we can come back to this later
There was a problem hiding this comment.
Just realized that the await here works fine in the context of a notebook runtime (where there's already an event loop), but I'm not sure Python would even let the app being served (SyntaxError?).
An other approach inspired by the simpler example Demetris added would be to instantiate the class with a timeout and pass it down to the callback.
|
@amaloney FYI in 3992c20 I cleared the notebook cell outputs. On the other HoloViz repos this is usually done automatically via a pre-commit hook Simon maintains (https://github.com/hoxbro/clean_notebook). This repo is different as it needs to allow some example notebooks to be committed with their full output, to deal with examples that can't run on the CI (need GPU, ultra-large datasets, too long computational time, etc.). So, we just clear the standard notebooks manually before committing them. If not, the test suite on the CI will detect they contain outputs and fail. |
|
@maximlt so don't clear the outputs after addressing the PR comments? |
|
@maximlt add other items as you see fit. I don't think the following are too time consuming to implement.
|
The other way around, please always clear the outputs unless it's a very special example :)
Perhaps a good approach would be to come up with a minimalist example (e.g. 1 curve from random data) using these 4 building blocks (Buffer, DynamicMap, etc.). I'm personally curious about |
|
Your changes were successfully integrated in the dev site, make sure to review the pages of the projects you touched before merging this PR. |
|
Your changes were successfully integrated in the dev site, make sure to review the pages of the projects you touched before merging this PR. |
|
Your changes were successfully integrated in the dev site, make sure to review the pages of the projects you touched before merging this PR. |
I opted to add a minimap streaming app towards the beginning with some random data. I also included extensive explanation later on about each of the implementation steps in question. I am choosing not to refactor the app to strip out the |
|
Your changes were successfully integrated in the dev site, make sure to review the pages of the projects you touched before merging this PR. |
|
For future extensions to this example (not this PR):
|
|
Your changes were successfully integrated in the dev site, make sure to review the pages of the projects you touched before merging this PR. |
|
Your changes were successfully integrated in the dev site, make sure to review the pages of the projects you touched before merging this PR. |
|
Your changes were successfully integrated in the dev site, make sure to review the pages of the projects you touched before merging this PR. |
Example to demonstrate streaming multichannel timeseries data.
Can stream live from CPUs as a way to confirm that the streaming machinery is working.
Can also stream from an LSL stream (mock from EEG file) which is used throughout research for signal acquisition.